fix(session): avoid tty control in Treeland#101
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts UserSession childModifier TTY handling so Treeland single-mode sessions no longer open or take control of a VT, while preserving VT-leak behavior for other non-X11 sessions to avoid races and SIGHUP-induced apparent crashes. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In the Treeland branch
auth->type != Display::TreelandleavesvtFd = -1andttyStringempty; double‑check subsequent logic inchildModifier()doesn’t assume a valid fd or nonempty path (it may be clearer to early‑return for Treeland or to guard all VT operations on avtFd >= 0check). - The nested
if (auth->type != Display::Treeland)insideif (auth->type != Display::X11)reads a bit inverted; consider explicitly enumerating the display types that should open a VT (e.g., aswitchonauth->type) to make the control flow and future extension to new display backends safer.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the Treeland branch `auth->type != Display::Treeland` leaves `vtFd = -1` and `ttyString` empty; double‑check subsequent logic in `childModifier()` doesn’t assume a valid fd or nonempty path (it may be clearer to early‑return for Treeland or to guard all VT operations on a `vtFd >= 0` check).
- The nested `if (auth->type != Display::Treeland)` inside `if (auth->type != Display::X11)` reads a bit inverted; consider explicitly enumerating the display types that should open a VT (e.g., a `switch` on `auth->type`) to make the control flow and future extension to new display backends safer.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
1. Skip opening the VT for Treeland single-mode user sessions, because the running Treeland compositor already owns the display server side. 2. Keep the VT-as-stdin and TIOCSCTTY path only for non-X11 sessions that start their own display server, such as standalone Wayland sessions. 3. Fix a fast-login race where dde-session became the controlling tty leader of /dev/tty2 while Treeland and dde-seatd were still completing VT transition. 4. The visible symptom was that dde-session appeared to crash shortly after login, while the real reason was a kernel SIGHUP caused by tty hangup. Log: Prevent Treeland single-mode dde-session from taking the VT as controlling tty. fix(session): 避免 Treeland 接管 tty 1. Treeland single-mode 用户会话不再打开 VT,因为显示服务端已经由正在运行的 Treeland compositor 持有。 2. 仅保留自带显示服务端的非 X11 会话继续走 VT stdin 和 TIOCSCTTY 路径,例如独立 Wayland 会话。 3. 修复快速登录时 dde-session 成为 /dev/tty2 控制终端 leader,而 Treeland 和 dde-seatd 仍在完成 VT 切换导致的时序问题。 4. 表现现象是登录后一两秒 dde-session 看起来像 crash,实际原因是 tty hangup 触发了内核发送的 SIGHUP。 Log: 避免 Treeland single-mode 下 dde-session 将 VT 作为控制终端。 PMS: TASK-390987
deepin pr auto review这段代码的修改主要是增加了对Treeland会话类型的特殊处理。让我对这段代码进行审查:
改进建议:
if (auth->type != Display::Treeland) {
ttyString = TtyUtils::path(auth->tty);
vtFd = ::open(qPrintable(ttyString), O_RDWR | O_NOCTTY);
if (vtFd == -1) {
qCWarning(log) << "Failed to open VT" << ttyString;
// 可以考虑添加错误处理逻辑
}
}
if (auth->type != Display::Treeland) {
QString ttyString = TtyUtils::path(auth->tty);
int vtFd = ::open(qPrintable(ttyString), O_RDWR | O_NOCTTY);
// ... 使用ttyString和vtFd ...
}
if (auth->type != Display::Treeland) {
qCDebug(log) << "Opening VT for non-Treeland session:" << auth->tty;
QString ttyString = TtyUtils::path(auth->tty);
int vtFd = ::open(qPrintable(ttyString), O_RDWR | O_NOCTTY);
if (vtFd == -1) {
qCWarning(log) << "Failed to open VT" << ttyString;
}
}总体来说,这段代码的修改是合理的,增加了对特殊会话类型的支持,提高了代码的健壮性。上述建议可以进一步提高代码的可靠性和可维护性。 |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: mhduiy, zccrs The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
• 问题现象:
Treeland single-mode 下,快速在 greeter 输入密码进入 session 时,dde-session 会在启动后一两秒退出,ddm 日志显示 session crash。慢一点输入密码时不容易复现。
原因:
原逻辑把所有非 X11 session 都当成需要接管 VT 的会话处理,会让 dde-session 打开 /dev/tty2 并通过 TIOCSCTTY 成为该 VT 的控制终端 leader。
但 Treeland single-mode 下,真正持有显示服务端能力的是已经运行的 Treeland compositor,dde-session 只是用户会话进程,不应该接管 VT。快速登录时,dde-session 过早成为 /dev/tty2 的控制终端 leader,后续 Treeland / dde-seatd 完成 VT 切换时触发 tty hangup,内
核向 dde-session 发送 SIGHUP,ddm 因此看到 session crash。
解法:
在 UserSession::childModifier() 中区分 Treeland single-mode 和普通 Wayland:
修复后,dde-session 仍通过 XDG_VTNR=2 归属用户 VT,但进程本身不再占有 /dev/tty2,因此不会再被 tty hangup 的 SIGHUP 杀掉。
Log: Prevent Treeland single-mode dde-session from taking the VT as controlling tty.
fix(session): 避免 Treeland 接管 tty
Log: 避免 Treeland single-mode 下 dde-session 将 VT 作为控制终端。
Summary by Sourcery
Adjust session VT handling to avoid Treeland single-mode sessions becoming the controlling TTY leader when a compositor is already active.
Bug Fixes:
Enhancements: